Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add photo api #133

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add photo api #133

wants to merge 1 commit into from
+609 −7

Conversation

izouxv
Copy link
Contributor

@izouxv izouxv commented Oct 27, 2022

add photo api

@izouxv
Copy link
Contributor Author

izouxv commented Oct 27, 2022

@twiss

@izouxv
Copy link
Contributor Author

izouxv commented Jan 4, 2023

@twiss can you merge this pr. thanks

@izouxv
Copy link
Contributor Author

izouxv commented Sep 2, 2023

@twiss thank you

@dpifke dpifke mentioned this pull request Aug 20, 2024
Open
twiss
twiss reviewed Sep 2, 2024
View reviewed changes
Copy link
Member

@twiss twiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the long delay in reviewing. It was a large one 😅

openpgp/keys.go Outdated Show resolved Hide resolved
openpgp/keys.go Outdated Show resolved Hide resolved
openpgp/keys.go Outdated Show resolved Hide resolved
openpgp/keys.go Outdated Show resolved Hide resolved
openpgp/keys.go Outdated Show resolved Hide resolved
openpgp/uat_test.go Outdated Show resolved Hide resolved
openpgp/uat_test.go Outdated Show resolved Hide resolved
openpgp/read_write_test_data.go Outdated Show resolved Hide resolved
openpgp/read_write_test_data.go Outdated Show resolved Hide resolved
openpgp/uat_test.go Outdated Show resolved Hide resolved
@izouxv izouxv force-pushed the addphoto branch 2 times, most recently from 50c23b2 to 7a01e13 Compare September 2, 2024 23:55
@izouxv
Copy link
Contributor Author

izouxv commented Sep 3, 2024

@twiss please check again. thank you

@twiss
Copy link
Member

twiss commented Sep 3, 2024

@izouxv You ignored all the comments that are hidden by default, you need to click "Load more..." or check the Files tab.

twiss
twiss reviewed Sep 3, 2024
View reviewed changes
openpgp/keys.go Outdated Show resolved Hide resolved
openpgp/keys.go Outdated Show resolved Hide resolved
@izouxv
Copy link
Contributor Author

izouxv commented Sep 3, 2024

@twiss please check again. thank you

@twiss
Copy link
Member

twiss commented Sep 3, 2024

This comment was not addressed. I know it didn't exist at the time this PR was opened, so I apologize for the churn, but all new functionality should go in the openpgp/v2 package. Please move the changes (except those in openpgp/packet) there, if you can.

@izouxv
Copy link
Contributor Author

izouxv commented Sep 3, 2024

@twiss please check again. thank you

@izouxv
Copy link
Contributor Author

izouxv commented Sep 3, 2024

This comment was not addressed. I know it didn't exist at the time this PR was opened, so I apologize for the churn, but all new functionality should go in the openpgp/v2 package. Please move the changes (except those in openpgp/packet) there, if you can.

ok

twiss
twiss reviewed Sep 5, 2024
View reviewed changes
openpgp/packet/public_key.go Outdated Show resolved Hide resolved
openpgp/packet/public_key.go Outdated Show resolved Hide resolved
openpgp/packet/public_key.go Outdated Show resolved Hide resolved
openpgp/packet/userattribute.go Show resolved Hide resolved
openpgp/v2/keys.go Outdated Show resolved Hide resolved
openpgp/v2/keys.go Outdated Show resolved Hide resolved
openpgp/packet/signature.go Outdated Show resolved Hide resolved
openpgp/packet/signature.go Outdated Show resolved Hide resolved
@izouxv
Copy link
Contributor Author

izouxv commented Sep 8, 2024

@twiss please check again. thank you

twiss
twiss reviewed Sep 9, 2024
View reviewed changes
openpgp/packet/public_key.go Outdated Show resolved Hide resolved
openpgp/packet/signature.go Outdated Show resolved Hide resolved
openpgp/v2/keys.go Outdated Show resolved Hide resolved
openpgp/v2/uat.go Outdated Show resolved Hide resolved
openpgp/v2/user.go Show resolved Hide resolved
@izouxv
Copy link
Contributor Author

izouxv commented Sep 10, 2024

@twiss please check again. thank you

@twiss twiss force-pushed the addphoto branch 2 times, most recently from 8265064 to c3b1f43 Compare September 11, 2024 10:04
@izouxv
Copy link
Contributor Author

izouxv commented Sep 13, 2024

@twiss HI,When can this be merged?

twiss
twiss reviewed Sep 16, 2024
View reviewed changes
openpgp/v2/keys.go Outdated Show resolved Hide resolved
@twiss
Copy link
Member

twiss commented Sep 16, 2024

In addition to the above, could you please write a brief summary of the changes and additions to the API in the PR description, so that I can include it in the commit message? Currently it's very vague.

@izouxv
Copy link
Contributor Author

izouxv commented Sep 19, 2024

In addition to the above, could you please write a brief summary of the changes and additions to the API in the PR description, so that I can include it in the commit message? Currently it's very vague.

ok

@izouxv
Copy link
Contributor Author

izouxv commented Sep 25, 2024

@twiss please check again. thank you

@twiss
Copy link
Member

twiss commented Sep 25, 2024

@izouxv where did you add the description? :s

@izouxv
Copy link
Contributor Author

izouxv commented Sep 25, 2024

@twiss please check again. thank you.
pr commit is: [feat](feat: Add AddPhotos to v2 api.)

@izouxv
Copy link
Contributor Author

izouxv commented Oct 3, 2024

@twiss please check again. thank you.

@twiss
Copy link
Member

twiss commented Oct 3, 2024

Sorry, I meant more like: could you write a short description of the functions you added and what they're for? See 8b3893e for a random (unrelated) example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@twiss twiss twiss left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@izouxv @twiss